Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add MongoDB Assistant Tools #472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nlarew wants to merge 11 commits into mongodb-js:main
base: main
Choose a base branch
Loading
from nlarew:assistant-tool
Open

Conversation

Copy link

@nlarew nlarew commented Aug 22, 2025

Proposed changes

(EAI-1266) Knowledge Search on MCP

Adds a new class of Assistant tools for interacting with the MongoDB Assistant (AI Chatbot + Search).

This PR scaffolds the generic AssistantToolBase class as well as two initial tools:

  • list_knowledge_sources - pulls a list of all available data sources. Useful for knowing what data is available and for filtering the search_knowledge
  • search_knowledge - runs a natural language search against the MongoDB Assistant knowledge base. Returns content chunks for the most relevant results.

Checklist

@Copilot Copilot AI review requested due to automatic review settings August 22, 2025 19:27
@nlarew nlarew requested a review from a team as a code owner August 22, 2025 19:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces MongoDB Assistant tools for the MCP server, enabling natural language search and knowledge source discovery. It adds integration with the MongoDB Assistant AI chatbot and search service.

  • Scaffolds a new AssistantToolBase class for MongoDB Assistant tool interactions
  • Implements two initial tools: list_knowledge_sources and search_knowledge
  • Adds comprehensive test coverage with mocked API responses

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/common/config.ts Adds assistantBaseUrl configuration option
src/tools/tool.ts Extends ToolCategory type to include "assistant"
src/tools/assistant/assistantTool.ts Base class for Assistant tools with common functionality
src/tools/assistant/list_knowledge_sources.ts Tool for listing available knowledge sources
src/tools/assistant/search_knowledge.ts Tool for searching the knowledge base
src/tools/assistant/tools.ts Exports array of Assistant tools
src/server.ts Registers Assistant tools with the server
tests/integration/helpers.ts Updates response element types to support metadata
tests/integration/tools/assistant/assistantHelpers.ts Test utilities for Assistant tool testing
tests/integration/tools/assistant/listKnowledgeSources.test.ts Integration tests for list knowledge sources tool
tests/integration/tools/assistant/searchKnowledge.test.ts Integration tests for search knowledge tool

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 21 to 26
this.baseUrl = new URL(config.assistantBaseUrl);
const serverVersion = packageInfo.version;
this.requiredHeaders = {
"x-request-origin": "mongodb-mcp-server",
"user-agent": serverVersion ? `mongodb-mcp-server/v${serverVersion}` : "mongodb-mcp-server",
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we try to use the apiClient to make this call? do you need specific headers or can you leverage that to make requests? Check getIpInfo in that file for a customized call

Also, is this page not authenticated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the apiClient is specifically for talking to Atlas/MMS APIs? The knowledge APIs are a distinct server so I don't think it will work for this.

do you need specific headers

Yeah we must include, at minimum, a user-agent + origin or x-request-origin header

is this page not authenticated

correct - the knowledge API is not authenticated

headers: this.requiredHeaders,
});
if (!response.ok) {
throw new Error(`Failed to list knowledge sources: ${response.statusText}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead return content instead of throwing an exception? we've been changing this across the code. it's better to have the tool return a response but with the proper messaging

nlarew reacted with thumbs up emoji
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Updated for both assistant tools.


export type OperationType = "metadata" | "read" | "create" | "delete" | "update" | "connect";
export type ToolCategory = "mongodb" | "atlas";
export type ToolCategory = "mongodb" | "atlas"|"assistant";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're here, can we also update the readme.md? there we call out all the tool categories

nlarew reacted with thumbs up emoji
Comment on lines 31 to 53
const searchEndpoint = new URL("content/sources", this.baseUrl);
const response = await fetch(searchEndpoint, {
method: "GET",
headers: this.requiredHeaders,
});
if (!response.ok) {
const message = `Failed to list knowledge sources: ${response.statusText}`;
this.session.logger.debug({
id: LogId.assistantListKnowledgeSourcesError,
context: "assistant-list-knowledge-sources",
message,
});
return {
content: [
{
type: "text",
text: message,
},
],
isError: true,
};
}
const { dataSources } = listDataSourcesResponseSchema.parse(await response.json());
Copy link
Collaborator

@nirinchev nirinchev Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we abstract this and move it to the base tool? Essentially, expose a function like:

protected function fetchKnowledgeBase<T>(method: "GET" | "POST", path: string, body?: unknown): Promise<T>

that calls fetch, parses the response, logs errors, etc. Additionally, we tend to prefer working with pure typescript types rather than zod schemas for API calls. Those are easier to reason about and more succinct to type.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call on moving the API call up one level of abstraction - done.

I don't follow the "no zod on API calls" logic. A network boundary is exactly where validation libraries like Zod are most useful - otherwise I have to assert the type (unsafe) or roll my own validation (clunkier than zod).

Copy link
Collaborator

@nirinchev nirinchev Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - the main thing here is that we're in control of the API, so a simple type assertion is reasonable.

Copy link
Author

@nlarew nlarew Sep 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well probably not how I'd handle it but also not my repo! Updated to remove the zod parse (and corresponding tests that check if we handle a malformed response) and replaced it with a vanilla assertion.

(Note - using as instead of : assertion to be more explicit that this is escape hatching. Both are equivalent since response.json() returns Promise<any>.)

if (args.method === "POST") {
headers.set("Content-Type", "application/json");
}
return await fetch(endpoint, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't use Node.js fetch, but customFetch configured like in the Atlas Client:

// createFetch assumes that the first parameter of fetch is always a string
// with the URL. However, fetch can also receive a Request object. While
// the typechecking complains, createFetch does passthrough the parameters
// so it works fine.
private static customFetch: typeof fetch = createFetch({
useEnvironmentVariableProxies: true,
}) as unknown as typeof fetch;

The reason is that the customFetch supports enterprise proxies, which are relevant for customers with specific security requirements.

Comment on lines +53 to +62
return {
content: dataSources.map(({ id, type, versions }) => ({
type: "text",
text: id,
_meta: {
type,
versions,
},
})),
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should convert this into a string, similar to what we do in:

return {
content: formatUntrustedData(
this.generateMessage({
collection,
queryResultsCount,
documents: cursorResults.documents,
appliedLimits: [limitOnFindCursor.cappedBy, cursorResults.cappedBy].filter((limit) => !!limit),
}),
cursorResults.documents.length > 0 ? EJSON.stringify(cursorResults.documents) : undefined
),
};

And also we should consider the information in the knowledge list untrusted.

}
const { results } = (await response.json()) as SearchKnowledgeResponse;
return {
content: results.map(({ text, metadata }) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, convert to string and treat it as untrusted data. We have a function for this:

https://github.com/mongodb-js/mongodb-mcp-server/blob/main/src/tools/tool.ts#L293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmruiz kmruiz kmruiz left review comments

@nirinchev nirinchev nirinchev left review comments

@blva blva blva left review comments

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@mongodben mongodben mongodben left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /